-
Notifications
You must be signed in to change notification settings - Fork 13
Add response types for KEYS and SCAN commands #244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Natan Rolnik <[email protected]>
aafc50e
to
785b46d
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #244 +/- ##
=======================================
Coverage ? 39.73%
=======================================
Files ? 94
Lines ? 15774
Branches ? 0
=======================================
Hits ? 6268
Misses ? 9506
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
public let keys: [ValkeyKey] | ||
|
||
public init(fromRESP token: RESPToken) throws { | ||
let (cursor, keys) = try token.decodeArrayElements(as: (Int, [ValkeyKey]).self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use (self.cursor, self.keys) = ...
. You don't need the temporary variables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor change
@natanrolnik I was thinking about this. In general I have not been converting paying for the extra allocation if they want by converting to an Array let scanResponse = try await client.scan(cursor: 0)
let keys = try scanResponse.keys.decode(as: [ValkeyKey].self) or they can avoid the array allocation with let scanResponse = try await client.scan(cursor: 0)
for token in scanResponse.keys {
let key = try ValkeyKey(respToken: token)
...
} |
Signed-off-by: Natan Rolnik <[email protected]>
@adam-fowler got it - done ✅ I was about to write that we needed more docs on |
While working on a sample project, I found it was really hard to parse the response of the
SCAN
andKEYS
commands.After talking to @adam-fowler, he told me about the
RESPTokenDecodable
protocol, which makes it really easy to implement type safe responses, so I added them.